server/streamable_http.go (1)

249-251: LGTM with minor edge case consideration.

The updated sampling response detection logic correctly differentiates from ping responses. However, consider handling the edge case where both Result and Error are nil - such messages would bypass both ping and sampling detection.

Consider adding explicit validation:

 isSamplingResponse := jsonMessage.Method == "" && jsonMessage.ID != nil &&
-	(jsonMessage.Result != nil || jsonMessage.Error != nil)
+	(jsonMessage.Result != nil || jsonMessage.Error != nil) &&
+	!isPingResponse

server/streamable_http_test.go (1)

897-990: Improve test coverage and assertions.

The test structure is good and covers the main scenarios, but could be enhanced:

1. Missing positive assertions: Tests only verify that sampling errors don't occur, but don't confirm that ping responses are actually handled correctly.

2. Unrealistic scenario: The third test case with empty error may not represent a realistic ping/pong scenario according to the MCP spec.

Consider these improvements:

+	// First verify a real sampling response still triggers the error
+	t.Run("Real sampling response should still require session ID", func(t *testing.T) {
+		samplingResponse := map[string]any{
+			"jsonrpc": "2.0",
+			"id":      126,
+			"result":  map[string]any{"content": "test"},
+		}
+		
+		resp, err := postJSON(server.URL, samplingResponse)
+		if err != nil {
+			t.Fatalf("Failed to send sampling response: %v", err)
+		}
+		defer resp.Body.Close()
+		
+		if resp.StatusCode == http.StatusOK {
+			t.Error("Expected sampling response to fail without session ID")
+		}
+	})

Also consider verifying response content more thoroughly:

 		if resp.StatusCode != http.StatusOK {
 			t.Errorf("Expected status 200 for pong response, got %d. Body: %s", resp.StatusCode, bodyStr)
 		}
+		
+		// Verify response body is empty (as expected for ping responses)
+		if len(bodyBytes) > 0 {
+			t.Errorf("Expected empty response body for ping, got: %s", bodyStr)
+		}

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

server/streamable_http.go (1)

267-274: Explicitly acknowledge pong and make detection resilient to meta-only payloads

• Early return risks middleware/proxies misinterpreting the response. Send 200 OK explicitly.
• Treat result/error objects containing only "_meta" as empty to match tolerant clients per prior feedback.

Apply:

-	// detect empty ping response, skip session ID validation
-	isPingResponse := jsonMessage.Method == "" && jsonMessage.ID != nil &&
-		(isJSONEmpty(jsonMessage.Result) && isJSONEmpty(jsonMessage.Error))
-
-	if isPingResponse {
-		return
-	}
+	// detect pong response (no method, has id, and empty/omitted result or error)
+	isPingResponse := jsonMessage.Method == "" && jsonMessage.ID != nil &&
+		(isJSONEmpty(jsonMessage.Result) || isJSONEmpty(jsonMessage.Error))
+
+	if isPingResponse {
+		w.WriteHeader(http.StatusOK)
+		return
+	}

And update isJSONEmpty as suggested below to consider {"_meta": ...} empty as well. Also, please add a small debug/info log when a pong is detected for observability. Would you like me to add a follow-up commit with tests for the {"_meta":...} case?

server/streamable_http.go (1)

951-997: Replace byte-scanning with parse-based emptiness; drop unnecessary imports

• Current byte/rune scanning is brittle and doesn’t recognize {"_meta":...} as empty.
• JSON whitespace is ASCII; using unicode.IsSpace on bytes is unnecessary.
• Simplify by unmarshalling objects and checking keys.

Apply:

-// isJSONEmpty reports whether the provided JSON value is "empty":
-//   - null
-//   - empty object: {}
-//   - empty array: []
-// It also treats nil/whitespace-only input as empty.
-// It does NOT treat 0, false, "" or non-empty composites as empty.
-func isJSONEmpty(data json.RawMessage) bool {
-	if len(data) == 0 {
-		return true
-	}
-
-	trimmed := bytes.TrimSpace(data)
-	if len(trimmed) == 0 {
-		return true
-	}
-
-	switch trimmed[0] {
-	case '{':
-		if len(trimmed) == 2 && trimmed[1] == '}' {
-			return true
-		}
-		for i := 1; i < len(trimmed); i++ {
-			if !unicode.IsSpace(rune(trimmed[i])) {
-				return trimmed[i] == '}'
-			}
-		}
-	case '[':
-		if len(trimmed) == 2 && trimmed[1] == ']' {
-			return true
-		}
-		for i := 1; i < len(trimmed); i++ {
-			if !unicode.IsSpace(rune(trimmed[i])) {
-				return trimmed[i] == ']'
-			}
-		}
-
-	case '"': // treat "" as not empty
-		return false
-
-	case 'n': // null
-		return len(trimmed) == 4 &&
-			trimmed[1] == 'u' &&
-			trimmed[2] == 'l' &&
-			trimmed[3] == 'l'
-	}
-	return false
-}
+// isJSONEmpty reports whether the provided JSON value is "empty":
+//   - null
+//   - empty object: {}
+//   - object with only "_meta" key
+//   - omitted (nil)
+// NOTE: We intentionally do NOT treat []/""/0/false as empty.
+func isJSONEmpty(data json.RawMessage) bool {
+	if len(data) == 0 {
+		return true
+	}
+	// null
+	if string(data) == "null" {
+		return true
+	}
+	// object {}
+	var obj map[string]json.RawMessage
+	if err := json.Unmarshal(data, &obj); err == nil {
+		if len(obj) == 0 {
+			return true
+		}
+		if len(obj) == 1 {
+			_, onlyMeta := obj["_meta"]
+			return onlyMeta
+		}
+	}
+	return false
+}

And clean up imports:

-import (
-	"bytes"
+import (
 	"context"
 	"encoding/json"
 	"fmt"
 	"io"
 	"mime"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
-	"unicode"

server/streamable_http_test.go (1)

897-991: Good coverage; add a meta-only pong case

Add a subtest for {"result":{"_meta":{...}}} to ensure tolerant handling, and to lock in the helper behavior.

Suggested addition:

 	t.Run("Pong response with empty result should not be treated as sampling response", func(t *testing.T) {
 		// ...
 	})
 
+	t.Run("Pong response with only _meta in result should not be treated as sampling response", func(t *testing.T) {
+		pongResponse := map[string]any{
+			"jsonrpc": "2.0",
+			"id":      126,
+			"result": map[string]any{
+				"_meta": map[string]any{"client": "test"},
+			},
+		}
+		resp, err := postJSON(server.URL, pongResponse)
+		if err != nil {
+			t.Fatalf("Failed to send pong response: %v", err)
+		}
+		defer resp.Body.Close()
+		if resp.StatusCode != http.StatusOK {
+			bodyBytes, _ := io.ReadAll(resp.Body)
+			t.Fatalf("Expected status 200, got %d. Body: %s", resp.StatusCode, string(bodyBytes))
+		}
+	})

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro